Conversation
This change: a) Adds a button to toggle the audio stream. This is needed since most browsers require the audio play action to be run in a user event handler, so having the toggle audio be an explicit button achieves that (and is also to avoid annoying users). b) Drops support for the QEMU audio extension and instead uses the Repl.it Audio messages, which allows for explicit negotiation for audio compression. c) Makes the audio library more robust and now works in Chrome and Firefox.
|
This PR is getting big. |
|
note: the failures are somewhat expected since the CI setup in this fork isn't quite the same as the one upstream. |
| document.getElementById('noVNC_audio_button') | ||
| .classList.remove("noVNC_hidden"); | ||
| document.getElementById('noVNC_audio_button') | ||
| .classList.remove("noVNC_selected"); |
There was a problem hiding this comment.
How often is updateCapabilities called? Should we always remove this selected class?
There was a problem hiding this comment.
at most four times per connection: once after the connection is established (to clear everything), once when the power capabilities are advertised by the server, once when the server says that it supports audio (only once if the user opted in), and once more when the connection is lost (to clean everything again for good measure).
it should be roughly fine to always attempt to remove the selected class since classList.remove() is a no-op if the class is not there, and other UI functions follow roughly the same pattern (like updatePowerButton(), another funciton that's called from updateCapabilities()).
which is a very long way of saying i guess it's fine?
There was a problem hiding this comment.
if they do this elsewhere, sounds good to me haha!
|
unbooping: approved |
4 participants
This change:
a) Adds a button to toggle the audio stream. This is needed since most
browsers require the audio play action to be run in a user event
handler, so having the toggle audio be an explicit button achieves
that (and is also to avoid annoying users).
b) Drops support for the QEMU audio extension and instead uses the
Repl.it Audio messages, which allows for explicit negotiation for
audio compression.
c) Makes the audio library more robust and now works in Chrome and
Firefox.
(cherry-pick of novnc#1525)